Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MIG][15.0] hr_holidays: migration script #3336

Merged
merged 2 commits into from
Mar 1, 2023

Conversation

tranngocson1996
Copy link

Migration done

@legalsylvain
Copy link
Contributor

/ocabot migration hr_holidays

Copy link
Contributor

@remi-filament remi-filament left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested OK on our dabatase migration.
Thank you @tranngocson1996

Copy link
Contributor

@remi-filament remi-filament left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double-checking holiday type configuration, I think there is a misunderstading on mapping allocation_type and allocation_validation_type (from v14) with requires_allocation, employee_requests and allocation_validation_type (from v15)

hr_holidays / hr.leave.allocation / interval_unit (selection) : DEL selection_keys: ['days', 'months', 'weeks', 'years']
hr_holidays / hr.leave.allocation / lastcall (date) : NEW hasdefault
hr_holidays / hr.leave.allocation / number_per_interval (float) : DEL
# NOTHING TO DO: handled by ORM
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be converted to an accrual plan instead, and link the allocation with it.

# DONE: map validate1 -> confirm

hr_holidays / hr.leave.allocation / taken_leave_ids (one2many) : NEW relation: hr.leave
hr_holidays / hr.leave.allocation / unit_per_interval (selection) : DEL selection_keys: ['days', 'hours']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one goes also to the accrual plan.

@sergio-teruel
Copy link

@tranngocson1996 Can you attend the comments? Thanks

@MiquelRForgeFlow
Copy link
Contributor

@pedrobaeza I am not able to recreate plans/levels, because not enough data. I see some vague map between hr.leave.allocation.unit_per_interval -> hr.leave.accrual.level.added_value_type and hr.leave.allocation.interval_unit -> hr.leave.accrual.level.frequency, but not enough old info to create those plans/levels with all those new fields.

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accrual plans are not created from previous existing allocations, as there's no 100% mapping between them, so that TODO is there, but being a very special case, let's continue with the merge.

@pedrobaeza pedrobaeza merged commit 01c8010 into OCA:15.0 Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants